feat: workflow list and trigger commands#68
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughAdds a new top-level ChangesWorkflow CLI feature
Workspace ID API change and caller updates
API client models update
Sequence Diagram(s)sequenceDiagram
actor CLI as "CLI (list)"
participant V as "Viper"
participant Client as "API Client"
participant API as "Backend API"
participant Out as "cliutil.HandleResponseOutput"
CLI->>V: read url, api-key, workspace
CLI->>Client: NewAPIKeyClientWithResponses(...)
CLI->>Client: GetWorkspaceID(ctx, workspace)
Client->>API: ListWorkflows(workspaceID, params)
API-->>CLI: workflows response
CLI->>Out: HandleResponseOutput(response)
sequenceDiagram
actor CLI as "CLI (trigger)"
participant Flags as "Flag parser"
participant V as "Viper"
participant Client as "API Client"
participant API as "Backend API"
participant Out as "cliutil.HandleResponseOutput"
CLI->>Flags: parse --input key=value flags
Flags-->>CLI: validated inputs map
CLI->>V: read url, api-key, workspace
CLI->>Client: NewAPIKeyClientWithResponses(...)
CLI->>Client: GetWorkspaceID(ctx, workspace)
Client->>API: CreateWorkflowRun(workspaceID, workflowID, body)
API-->>CLI: run creation response
CLI->>Out: HandleResponseOutput(response)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
internal/api/client.gen.go (1)
295-312: Confirm the minimum server version for the workflow contract changes.Lines 295-312 and 1317-1368 now only read/write
jobAgents, and Line 14994 only treats201as the typed create success. If this CLI can still target pre-migration API deployments, workflow reads will silently dropjobsand create calls will fall off the typed happy path. Please either version-gate these commands or keep dual-format handling in the API/spec until the rollout is complete.Also applies to: 1068-1073, 1317-1368, 10783-10787, 14993-14999
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/api/client.gen.go` around lines 295 - 312, The generated client now only serializes/deserializes CreateWorkflow.JobAgents and treats HTTP 201 as the sole typed create success, which will break compatibility with pre-migration APIs that use jobs or different success codes; update the client to either check the server version before using the new schema or implement dual-format handling: in functions that marshal/unmarshal CreateWorkflow and CreateWorkflowJobAgent (and any create workflow response handling that currently assumes 201), add logic to accept both "jobs" and "jobAgents" fields when reading workflows and to emit both formats (or choose format based on a negotiated/minimum server version); likewise, adjust the create response parsing to accept alternative success codes or fall back to untyped handling when the typed 201 path fails, using the CreateWorkflow and CreateWorkflowJobAgent types as the primary identifiers to locate relevant code paths.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmd/ctrlc/root/workflow/list.go`:
- Line 30: The call to client.GetWorkspaceID(cmd.Context(), workspace) can
return a zero UUID on failure; before calling client.ListWorkflows, check the
returned workspaceID for a zero/empty UUID and return an explicit error (or wrap
with fmt.Errorf) so we fail fast and don't send an invalid ID to ListWorkflows;
update the block around GetWorkspaceID and the subsequent call to ListWorkflows
to validate workspaceID and return early if it is the zero value.
- Around line 33-38: Replace the silent-ignore behavior for negative pagination
flags by validating the limit and offset variables before assigning to params:
check if limit < 0 or offset < 0 and return a CLI validation error (e.g.,
formatted error via fmt.Errorf) instead of falling through; only set
params.Limit and params.Offset when values are non-negative and valid. Update
the code paths around the existing assignments to params.Limit/params.Offset so
invalid inputs cause an early error return from the command handler (or pre-run)
and valid inputs continue to set the pointers as currently done.
In `@cmd/ctrlc/root/workflow/trigger.go`:
- Line 33: The code assigns workspaceID via client.GetWorkspaceID but doesn't
guard against a failed lookup returning a zero UUID; before making the trigger
call, check that workspaceID is not the zero value (compare against uuid.Nil or
the project's zero-UUID constant) and return a clear error (e.g., "workspace not
found" with the original workspace string) if it is zero to avoid calling the
trigger with an invalid identifier; update the function that calls
client.GetWorkspaceID and the subsequent trigger invocation to perform this
validation and short-circuit with an error.
- Around line 37-42: The parsing loop that uses strings.Cut(input, "=")
currently allows an empty key (e.g., "--input =foo"); after extracting key,
value, found in trigger.go validate that found is true and also that key != ""
(non-empty); if key is empty return a descriptive error (e.g., "invalid input
format %q, empty key, expected key=value") instead of assigning into the inputs
map so empty keys are rejected (refer to the variables key, value, found and the
inputs map where the assignment inputs[key] = value occurs).
---
Nitpick comments:
In `@internal/api/client.gen.go`:
- Around line 295-312: The generated client now only serializes/deserializes
CreateWorkflow.JobAgents and treats HTTP 201 as the sole typed create success,
which will break compatibility with pre-migration APIs that use jobs or
different success codes; update the client to either check the server version
before using the new schema or implement dual-format handling: in functions that
marshal/unmarshal CreateWorkflow and CreateWorkflowJobAgent (and any create
workflow response handling that currently assumes 201), add logic to accept both
"jobs" and "jobAgents" fields when reading workflows and to emit both formats
(or choose format based on a negotiated/minimum server version); likewise,
adjust the create response parsing to accept alternative success codes or fall
back to untyped handling when the typed 201 path fails, using the CreateWorkflow
and CreateWorkflowJobAgent types as the primary identifiers to locate relevant
code paths.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c68df1ae-9981-47c0-8b11-71cebac9d0be
📒 Files selected for processing (5)
cmd/ctrlc/root/root.gocmd/ctrlc/root/workflow/list.gocmd/ctrlc/root/workflow/trigger.gocmd/ctrlc/root/workflow/workflow.gointernal/api/client.gen.go
There was a problem hiding this comment.
🧹 Nitpick comments (1)
internal/api/client.go (1)
36-38: ⚡ Quick winImprove error message specificity for non-200 responses.
The error message "workspace %q not found" assumes a 404 response, but
resp.JSON200 == nilis true for any non-200 status (e.g., 403 Forbidden, 500 Internal Server Error). Consider including the status code or checking for specific response codes to provide more accurate error messages.💡 Proposed improvement
if resp.JSON200 == nil { - return uuid.Nil, fmt.Errorf("workspace %q not found", workspace) + return uuid.Nil, fmt.Errorf("failed to get workspace %q: HTTP %d", workspace, resp.StatusCode()) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/api/client.go` around lines 36 - 38, The error returned when resp.JSON200 == nil assumes a 404 but can occur for any non-200; update the handling in the function that checks resp.JSON200 (referencing resp.JSON200 and the workspace variable) to include the actual HTTP status code and/or more specific messages: e.g., if the underlying response status is 404 return "workspace %q not found", for other codes return an error that includes the status code (and optional response body or status text) so callers see whether it was 403, 500, etc.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@internal/api/client.go`:
- Around line 36-38: The error returned when resp.JSON200 == nil assumes a 404
but can occur for any non-200; update the handling in the function that checks
resp.JSON200 (referencing resp.JSON200 and the workspace variable) to include
the actual HTTP status code and/or more specific messages: e.g., if the
underlying response status is 404 return "workspace %q not found", for other
codes return an error that includes the status code (and optional response body
or status text) so callers see whether it was 403, 500, etc.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: efbe164e-3cf1-427d-a184-db9f9273d2bb
📒 Files selected for processing (17)
cmd/ctrlc/root/api/get/deployment/deployment.gocmd/ctrlc/root/api/get/release/release.gocmd/ctrlc/root/api/get/releasetargets/releasetargets.gocmd/ctrlc/root/api/get/resources/resources.gocmd/ctrlc/root/api/plan/version/version.gocmd/ctrlc/root/api/upsert/deploymentversion/deployment-version.gocmd/ctrlc/root/apply/cmd.gocmd/ctrlc/root/root.gocmd/ctrlc/root/sync/pipe/pipe.gocmd/ctrlc/root/ui/cmd.gocmd/ctrlc/root/workflow/list.gocmd/ctrlc/root/workflow/trigger.gointernal/api/client.gen.gointernal/api/client.gointernal/api/resolver/resolver.gointernal/resources/service.gopkg/resourceprovider/resourceprovider.go
✅ Files skipped from review due to trivial changes (1)
- internal/api/client.gen.go
🚧 Files skipped from review as they are similar to previous changes (3)
- cmd/ctrlc/root/root.go
- cmd/ctrlc/root/workflow/list.go
- cmd/ctrlc/root/workflow/trigger.go
Summary by CodeRabbit
New Features
Bug Fixes